lib: deserialize to native errors in error_serdes#58605
lib: deserialize to native errors in error_serdes#58605Renegade334 wants to merge 2 commits intonodejs:mainfrom
Conversation
e919a34 to
b353e31
Compare
| validateError(error, Error); | ||
| assert.notStrictEqual(error.cause, undefined); | ||
| validateError(error.cause, TypeError); | ||
| })); |
There was a problem hiding this comment.
Would be worth expanding this test to see if it works with the new SuppressedError and AggregateError types.
There was a problem hiding this comment.
I did consider AggregateError, although neither of these are explicitly handled currently.
node/lib/internal/error_serdes.js
Lines 46 to 48 in b353e31
At the moment, the logic traverses the prototype chain and ends up serialising these as Errors, so they end up with a prototype of Error.prototype. It doesn't really fall within the scope of this PR, but I can add a todo if desired?
There was a problem hiding this comment.
I've added a TODO to error_serdes for the uncovered constructors.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #58605 +/- ##
=======================================
Coverage 89.62% 89.62%
=======================================
Files 706 706
Lines 219137 219149 +12
Branches 41981 41985 +4
=======================================
+ Hits 196401 196419 +18
- Misses 14617 14619 +2
+ Partials 8119 8111 -8
🚀 New features to boost your workflow:
|
b353e31 to
71f1340
Compare
71f1340 to
e477957
Compare
Currently, errors deserialised by error_serdes are recreated using
Object.create()with the source error's prototype. The objects created using this technique are not considered native errors.This was always observable using
util.types.isNativeError(), but withError.isError()now being exposed, this becomes more readily apparent.Fixes: #48716